【高优先】重构 GM_registerMenuCommand 设计:稳定 menu item ID、同步更新、跨 iframe 一致性,行为对齐 Tampermonkey#820
【高优先】重构 GM_registerMenuCommand 设计:稳定 menu item ID、同步更新、跨 iframe 一致性,行为对齐 Tampermonkey#820cyfung1031 wants to merge 1 commit intoscriptscat:mainfrom
GM_registerMenuCommand 设计:稳定 menu item ID、同步更新、跨 iframe 一致性,行为对齐 Tampermonkey#820Conversation
There was a problem hiding this comment.
Pull Request Overview
本 PR 重构了 GM_registerMenuCommand 的设计以解决与 Tampermonkey 的行为差异问题,实现了稳定的菜单项 ID、即时同步更新、跨 iframe 一致性等功能。
- 将菜单项 ID 从数字改为基于内容的稳定唯一键(groupKey + contentEnvKey)
- 实现了即时菜单更新机制,无需关闭再打开 popup
- 统一了主框架和子框架之间的菜单显示与触发逻辑
Reviewed Changes
Copilot reviewed 13 out of 13 changed files in this pull request and generated 8 comments.
Show a summary per file
| File | Description |
|---|---|
| src/pages/store/AppContext.tsx | 增加类型定义和泛型支持,优化消息订阅机制 |
| src/pages/popup/App.tsx | 添加实时菜单更新订阅和排序逻辑,支持动态菜单变更 |
| src/pages/options/routes/Setting.tsx | 统一消息订阅的类型定义 |
| src/pages/options/routes/ScriptList/index.tsx | 统一消息订阅的类型定义 |
| src/pages/components/ScriptMenuList/index.tsx | 重构菜单项分组逻辑,实现 groupKey 合并显示 |
| src/app/service/service_worker/types.ts | 新增菜单相关类型定义,支持新的菜单项结构 |
| src/app/service/service_worker/popup.ts | 核心重构:实现稳定 contextMenu ID 和菜单同步机制 |
| src/app/service/service_worker/gm_api.ts | 更新 GM API 参数类型,适配新的菜单键结构 |
| src/app/service/service_worker/client.ts | 更新客户端接口,支持新的菜单参数结构 |
| src/app/service/queue.ts | 更新队列类型定义,支持新的菜单项键结构 |
| src/app/service/content/gm_api.ts | 重构客户端菜单注册逻辑,实现环境隔离和键管理 |
| src/app/service/content/gm_api.test.ts | 添加菜单注册和取消注册的测试用例 |
| src/app/cache.ts | 添加注释说明缓存返回值的边界情况 |
Comments suppressed due to low confidence (1)
src/app/service/service_worker/popup.ts:1
- 使用非空断言操作符
!可能导致运行时错误。应该检查data是否存在以及tabId是否为有效键,例如return data?.[sender.getExtMessageSender().tabId] || {}。
import { type IMessageQueue } from "@Packages/message/message_queue";
| ]; | ||
| return () => { | ||
| for (const unhook of unhooks) unhook(); | ||
| unhooks.length = 0; |
There was a problem hiding this comment.
清空数组的操作是多余的。由于 unhooks 是局部变量且在 useEffect 清理函数中只会执行一次,数组会在函数执行完毕后自动被垃圾回收,无需手动清空。
| unhooks.length = 0; |
| //@ts-ignore | ||
| b.enable - a.enable || |
There was a problem hiding this comment.
使用 @ts-ignore 忽略类型检查不是好的做法。应该通过类型断言或修复类型定义来解决类型错误,例如 Number(b.enable) - Number(a.enable)。
| //@ts-ignore | |
| b.enable - a.enable || | |
| Number(b.enable) - Number(a.enable) || |
| return () => { | ||
| isMounted = false; | ||
| for (const unhook of unhooks) unhook(); | ||
| unhooks.length = 0; |
There was a problem hiding this comment.
清空数组的操作是多余的。由于 unhooks 是局部变量且在 useEffect 清理函数中只会执行一次,数组会在函数执行完毕后自动被垃圾回收,无需手动清空。
| unhooks.length = 0; |
| ]; | ||
| return () => { | ||
| for (const unhook of unhooks) unhook(); | ||
| unhooks.length = 0; |
There was a problem hiding this comment.
清空数组的操作是多余的。由于 unhooks 是局部变量且在 useEffect 清理函数中只会执行一次,数组会在函数执行完毕后自动被垃圾回收,无需手动清空。
| unhooks.length = 0; |
| ]; | ||
| return () => { | ||
| for (const unhook of unhooks) unhook(); | ||
| unhooks.length = 0; |
There was a problem hiding this comment.
清空数组的操作是多余的。由于 unhooks 是局部变量且在 useEffect 清理函数中只会执行一次,数组会在函数执行完毕后自动被垃圾回收,无需手动清空。
| unhooks.length = 0; |
| return () => { | ||
| isMounted = false; | ||
| unsub(); | ||
| checkItems.clear(); |
There was a problem hiding this comment.
在 useEffect 清理函数中调用 checkItems.clear() 是多余的。Map 实例会在组件卸载后自动被垃圾回收,无需手动清理。
| checkItems.clear(); |
| // @ts-ignore | ||
| const exec = new ExecScript(script, "content", mockMessage, nilFn, envInfo); |
There was a problem hiding this comment.
在测试代码中使用 @ts-ignore 并不理想。应该通过正确的类型定义或类型断言来解决类型问题,例如使用 as any 或创建合适的 mock 类型。
| // @ts-ignore | ||
| const exec = new ExecScript(script, "content", mockMessage, nilFn, envInfo); |
There was a problem hiding this comment.
在测试代码中使用 @ts-ignore 并不理想。应该通过正确的类型定义或类型断言来解决类型问题,例如使用 as any 或创建合适的 mock 类型。
| // @ts-ignore | |
| const exec = new ExecScript(script, "content", mockMessage, nilFn, envInfo); | |
| const exec = new ExecScript(script, "content", mockMessage as any, nilFn, envInfo); |
|
似乎也没有太多有营养的内容,和之前差不多 |
|
Copilot Review完 |
详细见 #790 . 这PR只用来给Copilot Review
背景 / 动机
现有
GM_registerMenuCommand的行为与 Tampermonkey(TM)存在差异,导致:interval-n-xxxx)不显示。id的注册覆盖规则与期望不一致。本 PR 针对上述问题进行全面修订,使行为与 TM 对齐,并关闭 #787。
变更重点
稳定化浏览器 contextMenu 的项目 ID
interval-n-xxxx)。即时与可预期的动态更新
intervalChanging = true)时,popup 与 context menu 会即时反映,无需关闭再开。行为与 TM 一致。跨 frame(top 与 iframe)的一致性
interval-m-1001与interval-m-1002在不同 frame 被错误合并/拆分),现在结果与 TM 一致。相同
id的覆盖规则id重复注册时,只保留最新一个(举例:「MenuReg abc-2」覆盖「MenuReg abc-1」),并确保GM_unregisterMenuCommand(id)能正确移除对应项目。accessKey与返回值一致性accessKey更新与同一id下的覆盖;注册与反注册的返回值/语意保持一致。实作概要
GM_registerMenuCommand与GM_unregisterMenuCommand的内部资料结构与索引策略,确保 稳定 ID、单一事实来源 与 原子更新。id注册视为更新;解除注册支援以id、或以注册返回值识别。相容性 / 风险
id的脚本行为不变;多次注册同id的脚本现在会得到 TM 一致的覆盖效果。id多重并存」的非预期行为,升级后可能只看到最后一次注册的项目。建议在 release note 提醒。测试方案(可直接复制以下脚本验证)
将以下 Userscript 于任意页面执行,并依照注解切换三个测试开关:
checkSubFrameIdSequence:在有 iframe 的页面设为true,检查 top 与 iframe 的 ID/顺序。intervalChanging:设为true检查「每秒动态变更 menu」是否即时更新。skipClickCheck:预设false会引导互动测试;设true可跳过引导。此脚本已内嵌于 PR 讨论中(详见原 PR 内容)。
预期结果(关键观察点):
id注册仅显示最新一个条目(abc→ 只剩MenuReg abc-2)。id注册会同时显示两个条目,新增带accessKey的版本后,三者并存规则正确。GM_unregisterMenuCommand逐一移除后,对应条目正确消失。intervalChanging = true时,popup 与右键 context menu 即时刷新,不需重开。效能 / 可靠性
相关议题
GM_registerMenuCommand问题 #787检查清单
id覆盖 / 跨 frame)原PR: #790 "[高优先] 重新修订
GM_registerMenuCommand相关代码设计 by cyfung1031 · Pull Request #790 · scriptscat/scriptcat · GitHub"